Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add energy measurements support #140

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add energy measurements support #140

wants to merge 6 commits into from

Conversation

cappadokes
Copy link
Contributor

@cappadokes cappadokes commented Feb 24, 2022 •
edited
Loading

This PR introduces a new, --track-energy option for reporting energy consumption measurements instead of execution time, memory etc.

ATTENTION: --track-energy needs changes in pyperf in order to work, since pyperf is the real place where benchmarks are run.

@cappadokes cappadokes mentioned this pull request Feb 24, 2022
Open
gvanrossum
gvanrossum reviewed Mar 17, 2022
View reviewed changes
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR's title doesn't match what the code does (it seems to be adding a --track-energy flag).

pyperformance/data-files/requirements.txt
@@ -10,7 +10,7 @@ psutil==5.8.0
# via -r requirements.in
pyparsing==3.0.6
# via packaging
pyperf==2.3.0
pyperf @ file:///home/cappadokes/code/pyperf
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like something you changed locally, but that should be changed back before landing this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this is somewhat tricky:

  • for --track-energy to work, pyperf should also support energy measurements. There is no such pyperf release at the moment, but we are working on it.
  • until then, if a user wants to use --track-energy from pyperformance, she must indeed use a local pyperf in requirements.txt.

@cappadokes
Copy link
Contributor Author

This PR's title doesn't match what the code does (it seems to be adding a --track-energy flag).

To be precise, the code both fixes the --inherit-environ issue and it also adds a --track-energy flag. I apologize for not splitting this in 2 separate PRs, but the changes under discussion are partly interdependent (--track-energy needs --inherit-environ to work, though the converse does not hold).

I will soon follow up with a documentation commit for clarification. I will also change the PR's name so as to contain both points.

@cappadokes cappadokes changed the title Fix --inherit-environ issue Fix --inherit-environ issue, Add energy measurements support Mar 17, 2022
@cappadokes cappadokes mentioned this pull request Apr 11, 2022
Open
@ericsnowcurrently
Copy link
Member

@cappadokes, please put the --track-energy addition in a separate PR. We will merge the fix for --inherit-environ first.

@cappadokes cappadokes changed the title Fix --inherit-environ issue, Add energy measurements support Add energy measurements support Apr 27, 2022
@cappadokes
Copy link
Contributor Author

@cappadokes, please put the --track-energy addition in a separate PR. We will merge the fix for --inherit-environ first.

@ericsnowcurrently done. --inherit-environ is here now.

@cappadokes cappadokes mentioned this pull request Apr 27, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gvanrossum gvanrossum gvanrossum left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cappadokes @ericsnowcurrently @gvanrossum